Skip to content

Rewrite Visualisation#95

Merged
lispandfound merged 41 commits intomasterfrom
rewrite
Feb 21, 2025
Merged

Rewrite Visualisation#95
lispandfound merged 41 commits intomasterfrom
rewrite

Conversation

@lispandfound
Copy link
Contributor

@lispandfound lispandfound commented Feb 13, 2025

This PR introduces the source modelling plotting scripts from the source modelling repository, and then updates the Jenkins script to test these on 2p (soon Hypocentre) using docker containers. The Jenkinsfile can be used as a template to update the other repositories where integration or E2E testing is required.

Why Move to Visualisation?

Because the source modelling and velocity modelling plots increasingly depend on the workflow repository. They depend on the workflow repository because they require values from the realisation files to plot multi-segment SRFs and plot velocity models. The workflow repository depends on source and velocity modelling repos to perform scientific calculations. As a consequence we get circular dependencies that pip can't resolve easily. Hence, moving these scripts to a separate repository unpicks the circular dependencies.

How Does the Jenkinsfile Work?

I have updated Jenkins on 2p to the latest version, and installed the Docker Pipeline plugin. This plugin provides the ability to run our tests inside a container, like GitHub Actions. This simplifies the environment management of Jenkins for our end, as all we need to run our tests on the latest version of Python is

    agent {
        docker { image 'python:3.13' }
    }

Jenkins reads this block, pulls python 3.13 into the container, clones the repository and sets us up in a fresh environment. Then we just pip install the package with pip install -e . and run our code. I make use of pytest-xdist to parallelise the execution of tests. Once we migrate these integration and E2E tests to hypocentre we should see a large speed up in the tests and can run more complex tests with the increased hardware capabilities.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the following dependencies for consistency.

@lispandfound lispandfound dismissed github-actions[bot]’s stale review February 14, 2025 00:35

git extension check was broken

@lispandfound
Copy link
Contributor Author

I've added a new decorator in the visualisation.utils module that allows you to automatically extract docstring descriptions for typer-cli interfaces. So something like this in the old code.

@app.command(help="Plot segment magnitudes against the Leonard scaling relation.")
def plot_mw_contributions(
    srf_ffp: Annotated[
        Path, typer.Argument(help="Path to SRF file", exists=True, dir_okay=False)
    ],
    realisation_ffp: Annotated[
        Path,
        typer.Argument(help="Realisation filepath", dir_okay=False, exists=True),
    ],
    output_ffp: Annotated[
        Path, typer.Argument(help="Output plot path.", writable=True, dir_okay=False)
    ],
    dpi: Annotated[
        float, typer.Option(help="Output plot DPI (higher is better).")
    ] = 300,
    height: Annotated[float, typer.Option(help="Plot height (cm)", min=0)] = 10,
    width: Annotated[float, typer.Option(help="Plot width (cm)", min=0)] = 10,
) -> None:
    """Plot segment magnitudes against the Leonard scaling relation.

    Parameters
    ----------
    srf_ffp : Path
        Path to SRF file.
    realisation_ffp : Path
        Realisation filepath.
    output_ffp : Path
        Output plot path.
    dpi : float, default 300
        Output plot DPI (higher is better).
    height : float
        Height of plot (in cm).
    width : float
        Width of plot (in cm).
    """

becomes

@app.command()
@utils.from_docstring
def plot_mw_contributions(
    srf_ffp: Annotated[Path, typer.Argument(exists=True, dir_okay=False)],
    realisation_ffp: Annotated[Path, typer.Argument(exists=True, dir_okay=False)],
    output_ffp: Annotated[Path, typer.Argument(dir_okay=False)],
    dpi: Annotated[float, typer.Option()] = 300,
    height: Annotated[float, typer.Option(min=0)] = 10,
    width: Annotated[float, typer.Option(min=0)] = 10,
) -> None:
    """Plot segment magnitudes against the Leonard scaling relation.

    Parameters
    ----------
    srf_ffp : Path
        Path to SRF file.
    realisation_ffp : Path
        Realisation filepath.
    output_ffp : Path
        Output plot path.
    dpi : float, default 300
        Output plot DPI (higher is better).
    height : float
        Height of plot (in cm).
    width : float
        Width of plot (in cm).
    """

Notice that the redundant typer help descriptions are now extracted automatically from the docstring.

@lispandfound
Copy link
Contributor Author

Tests will now fail becaure they require ucgmsim/qcore#344.

Copy link
Contributor

@AndrewRidden-Harper AndrewRidden-Harper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

impressive work

Copy link
Member

@sungeunbae sungeunbae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent. visualisation vs visualization? Looks like you decided to stick to "s"? Oh, no. The repo name is still "z"....

@lispandfound
Copy link
Contributor Author

Excellent. visualisation vs visualization? Looks like you decided to stick to "s"? Oh, no. The repo name is still "z"....

Yeah I want to fix that, but Jenkins breaks because it is looking for "visualization". I will change it later when @AndrewRidden-Harper sets up the new Jenkins server.

@lispandfound
Copy link
Contributor Author

These tests also require: ucgmsim/velocity_modelling#11 because it forces numpy<2

@lispandfound
Copy link
Contributor Author

Note: I manually triggered a passing Jenkins build for this PR https://quakecoresoft.canterbury.ac.nz/jenkins/job/visualization/58/. It passes but doesn't update the PR check because I manually triggered it. I am overriding the check in this case, with this comment acting as proof of the successful build.

@lispandfound lispandfound merged commit 24118fe into master Feb 21, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants